Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Add sdk.Context and vm.EVM to precompiles #1433

Closed

Conversation

loredanacirstea
Copy link
Contributor

Description

Add context to precompiles: both Cosmos sdk.Context and vm.EVM.
vm.EVM provides block, tx, along with state access.
And if this PR ethereum/go-ethereum#26119 is merged, it will also provide current call/subcall context.

Related: #1131


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Add context to precompiles: both Cosmos `sdk.Context` and `vm.EVM`.
vm.EVM provides block, tx, along with state access.
And if this PR ethereum/go-ethereum#26119 is merged, it will also provide current call/subcall context.
@loredanacirstea loredanacirstea requested a review from a team as a code owner November 5, 2022 21:58
@loredanacirstea loredanacirstea requested review from MalteHerrmann and adisaran64 and removed request for a team November 5, 2022 21:58
@codecov
Copy link

codecov bot commented Nov 5, 2022

Codecov Report

Merging #1433 (4d620cd) into main (708e781) will increase coverage by 0.07%.
The diff coverage is 87.50%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1433      +/-   ##
==========================================
+ Coverage   57.59%   57.67%   +0.07%     
==========================================
  Files         108      108              
  Lines       10087    10095       +8     
==========================================
+ Hits         5810     5822      +12     
+ Misses       4031     4029       -2     
+ Partials      246      244       -2     
Impacted Files Coverage Δ
app/app.go 85.74% <33.33%> (-0.34%) ⬇️
x/evm/keeper/keeper.go 89.47% <100.00%> (ø)
x/evm/keeper/state_transition.go 82.80% <100.00%> (+0.04%) ⬆️
rpc/backend/account_info.go 76.81% <0.00%> (+2.83%) ⬆️
rpc/backend/utils.go 8.98% <0.00%> (+7.68%) ⬆️

// custom stateless precompiled smart contracts
customPrecompiles evm.PrecompiledContracts
// Precompile extensions
getPrecompilesExtended func(ctx sdk.Context, evm *vm.EVM) evm.PrecompiledContracts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could make this function take an address and return a singular precompile. Opens up more modularity.

func (EVM) ActivePrecompiles(rules params.Rules) []common.Address {
return vm.ActivePrecompiles(rules)
func (e EVM) ActivePrecompiles(rules params.Rules) []common.Address {
// TODO e.precompiles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires a geth fork right?

Copy link
Contributor Author

@loredanacirstea loredanacirstea Nov 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this ActivePrecompiles is not actually used now. I changed it when I was testing and forgot to revert the changes. For clarity, I will remove them. (will return to this when I have some time later today)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ethermint uses its own ActivePrecompiles function in state_transition.go. The only other places I've seen vm.ActivePrecompiles used in go-ethereum is tracer-related.

@fedekunze fedekunze marked this pull request as draft November 23, 2022 14:21
@fedekunze
Copy link
Contributor

marking this as Draft until we have more clarity on the final implementation

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants